security: avoid shell fallback in SLURM requeue#21751
security: avoid shell fallback in SLURM requeue#21751dfgvaetyj3456356-hash wants to merge 1 commit into
Conversation
dfgvaetyj3456356-hash
requested review from
ethanwharris,
justusschock and
tchaton
as code owners
|
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #21751 +/- ##
=========================================
- Coverage 87% 79% -8%
=========================================
Files 270 267 -3
Lines 23973 23912 -61
=========================================
- Hits 20748 18804 -1944
- Misses 3225 5108 +1883 |
|
Thanks for the CI run. It looks like the remaining red status is from a cancelled Ubuntu pl-cpu matrix job plus the pl-cpu guardian, while the other checks and Codecov report are green. I do not see a code failure from the changed SLURM requeue path. Could a maintainer rerun the cancelled job/guardian when convenient? Happy to adjust if a real failure appears. |
2 participants
What does this PR do?
This hardens SLURM auto-requeue handling by keeping the requeue call on the argv-based subprocess path and replacing the previous
assertvalidation with an explicit runtime check.The previous fallback retried
scontrol requeue <job_id>throughshell=Truewhenscontrolwas not found. Since the job id comes from the environment, this PR avoids ever turning that value into a shell command string. The validation also now usesre.fullmatch(...)and raises a normal exception so it is not removed when Python runs with optimizations.Test Plan
On Windows, the pytest file reports the SLURM-specific tests as skipped by the existing
RunIf(skip_windows=True)marker, so I also ran direct handler proofs outside the marker, including underpython -O, and confirmed invalid job ids are rejected beforesubprocess.callis invoked.